Skip to content

test: enhance lineage spec to cover all the missing cases#26796

Merged
chirag-madlani merged 29 commits intofix-column-filteringfrom
lineage-e2e-tests
Mar 30, 2026
Merged

test: enhance lineage spec to cover all the missing cases#26796
chirag-madlani merged 29 commits intofix-column-filteringfrom
lineage-e2e-tests

Conversation

@chirag-madlani
Copy link
Copy Markdown
Collaborator

@chirag-madlani chirag-madlani commented Mar 26, 2026

Describe your changes:

Fixes

I worked on ... because ...

Type of change:

  • Bug fix
  • Improvement
  • New feature
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation

Checklist:

  • I have read the CONTRIBUTING document.
  • My PR title is Fixes <issue-number>: <short explanation>
  • I have commented on my code, particularly in hard-to-understand areas.
  • For JSON Schema changes: I updated the migration scripts or explained why it is not needed.

Summary by Gitar

  • New comprehensive lineage test suite:
    • Added Lineagev1.spec.ts with 30+ test cases covering lineage creation, filtering, and interactions across 15+ entity types
    • Tests include domain, owner, tag, tier, service, and database filters with partial metadata assignment
  • Enhanced lineage utilities:
    • Added getDefaultAdminAPIContext helper for API context initialization
    • Extended getEntityTypeSearchIndexMapping to support 5 new entity types (Directory, File, Spreadsheet, Worksheet, StoredProcedure)
    • Updated verifyExportLineageCSV to accept generic entity arrays
  • Entity support additions:
    • Implemented patch method in MetricClass for entity metadata updates
    • Added lineage nodes for all missing entity types (Directory, File, Spreadsheet, Worksheet)
  • Test configuration update:
    • Modified playwright.config.ts to filter and run lineage-specific tests

This will update automatically on new commits.

@chirag-madlani chirag-madlani requested a review from a team as a code owner March 26, 2026 10:43
@github-actions github-actions bot added safe to test Add this label to run secure Github workflows on PRs UI UI specific issues labels Mar 26, 2026
SearchIndex: 'searchIndex',
ApiEndpoint: 'apiEndpoint',
Metric: 'metric',
['Store Procedure']: 'storedProcedure',
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Bug: Typo 'Store Procedure' should be 'Stored Procedure'

The search index mapping key ['Store Procedure'] is missing the 'd'. The rest of the codebase (e.g., importUtils.ts, class names) consistently uses 'Stored Procedure'. This mismatch will cause entity type lookups to fail for stored procedures, as getEntityTypeSearchIndexMapping('Stored Procedure') will return undefined.

Suggested fix:

Change:
    ['Store Procedure']: 'storedProcedure',
To:
    ['Stored Procedure']: 'storedProcedure',

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

Jest test Coverage

UI tests summary

Lines Statements Branches Functions
Coverage: 64%
64.84% (58266/89860) 44.66% (30783/68915) 47.66% (9222/19349)

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 26, 2026

🔴 Playwright Results — 83 failure(s), 21 flaky

✅ 3455 passed · ❌ 83 failed · 🟡 21 flaky · ⏭️ 208 skipped

Shard Passed Failed Flaky Skipped
🟡 Shard 1 453 0 2 2
🟡 Shard 2 625 0 5 32
🟡 Shard 3 638 0 3 26
🟡 Shard 4 608 0 2 47
🟡 Shard 5 599 0 1 67
🔴 Shard 6 532 83 8 34

Genuine Failures (failed on all attempts)

Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Table (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Container (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Topic (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Dashboard (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Mlmodel (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Pipeline (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Stored Procedure (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Search Index (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Data Model (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Api Endpoint (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Metric (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Directory (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - File (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Spreadsheet (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › verify create lineage for entity - Worksheet (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> table (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> container (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> topic (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> apiEndpoint (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> dashboard (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> dashboardDataModel (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> searchIndex (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for table -> mlModel (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> table (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> container (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> topic (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> apiEndpoint (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> dashboard (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> dashboardDataModel (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m
Pages/Lineage/DataAssetLineage.spec.ts › Column lineage for container -> searchIndex (shard 6)
Error: �[2mexpect(�[22m�[31mreceived�[39m�[2m).�[22mtoBeDefined�[2m()�[22m

Received: �[31mundefined�[39m

... and 53 more failures

🟡 21 flaky test(s) (passed on retry)
  • Pages/AuditLogs.spec.ts › should apply both User and EntityType filters simultaneously (shard 1, 1 retry)
  • Pages/UserCreationWithPersona.spec.ts › Create user with persona and verify on profile (shard 1, 1 retry)
  • Features/BulkEditEntity.spec.ts › Glossary (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should clear individual filter and update URL (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should show Service filter chip from URL (shard 2, 1 retry)
  • Features/ColumnBulkOperations.spec.ts › should navigate through pages (shard 2, 1 retry)
  • Features/Glossary/GlossaryHierarchy.spec.ts › should cancel move operation (shard 2, 1 retry)
  • Features/Permissions/GlossaryPermissions.spec.ts › Team-based permissions work correctly (shard 3, 1 retry)
  • Features/TestSuiteMultiPipeline.spec.ts › TestSuite multi pipeline support (shard 3, 1 retry)
  • Flow/ExploreDiscovery.spec.ts › Should display deleted assets when showDeleted is checked and deleted is not present in queryFilter (shard 3, 1 retry)
  • Pages/DataContractsSemanticRules.spec.ts › Validate Description Rule Is_Not_Set (shard 4, 1 retry)
  • Pages/Domains.spec.ts › Comprehensive domain rename with ALL relationships preserved (shard 4, 1 retry)
  • Pages/ExploreTree.spec.ts › Verify Database and Database Schema available in explore tree (shard 5, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage service filter selection (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › Verify lineage clear all filters (shard 6, 1 retry)
  • Pages/Lineage/LineageFilters.spec.ts › verify downstream count for all the entities (shard 6, 2 retries)
  • Pages/ODCSImportExport.spec.ts › Multi-object ODCS contract - object selector shows all schema objects (shard 6, 1 retry)
  • Pages/Users.spec.ts › Permissions for table details page for Data Consumer (shard 6, 1 retry)
  • Pages/Users.spec.ts › Check permissions for Data Steward (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Topic (shard 6, 1 retry)
  • VersionPages/EntityVersionPages.spec.ts › Directory (shard 6, 1 retry)

📦 Download artifacts

How to debug locally
# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

Comment on lines +455 to +458
test.beforeAll(async ({ browser }) => {
const { apiContext } = await getDefaultAdminAPIContext(browser);
await table.create(apiContext);
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Quality: Missing afterAll cleanup for table created in beforeAll

The newly added test.beforeAll in the 'Lineage Settings modal' block (DataAssetLineage.spec.ts:455-458) creates a TableClass entity but there is no corresponding test.afterAll to delete it. The same issue exists in PlatformLineage.spec.ts (line 35-38) where the table creation was moved to the top-level scope without adding cleanup.

While this follows the existing pattern in the file (no other describe block has afterAll either), it leaves orphaned test entities in the system between runs, which can cause flaky tests or data buildup in CI environments.

Suggested fix:

// DataAssetLineage.spec.ts — add after the beforeAll block:
test.afterAll(async ({ browser }) => {
  const { apiContext } = await getDefaultAdminAPIContext(browser);
  await table.delete(apiContext);
  await afterAction();
});

// Same pattern for PlatformLineage.spec.ts

Was this helpful? React with 👍 / 👎 | Reply gitar fix to apply this suggestion

@sonarqubecloud
Copy link
Copy Markdown

@sonarqubecloud
Copy link
Copy Markdown

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 30, 2026

❌ Playwright Lint Check Failed — ESLint + Prettier + Organise Imports

The following files have style issues that need to be fixed:
playwright/e2e/Pages/Lineage/DataAssetLineage.spec.ts playwright/e2e/Pages/Lineage/LineageControls.spec.ts playwright/e2e/Pages/Lineage/LineageFilters.spec.ts playwright/e2e/Pages/Lineage/LineageInteraction.spec.ts playwright/e2e/Pages/Lineage/LineageNodePagination.spec.ts playwright/e2e/Pages/Lineage/LineageRightPanel.spec.ts playwright/e2e/Pages/Lineage/PlatformLineage.spec.ts playwright/support/entity/MetricClass.ts playwright/utils/common.ts playwright/utils/lineage.ts

Fix locally (fast — changed files only):

cd openmetadata-ui/src/main/resources/ui
yarn ui-checkstyle:playwright:changed

Or to fix all playwright files: yarn ui-checkstyle:playwright

@gitar-bot
Copy link
Copy Markdown

gitar-bot bot commented Mar 30, 2026

Code Review ⚠️ Changes requested 9 resolved / 14 findings

Test enhancements for lineage specs with comprehensive entity creation and visibility checks. However, four important issues remain: typo 'Store Procedure' should be 'Stored Procedure', critical logic change from OR to AND in path preservation that alters filtering behavior, empty pagination state at nodeDepth=0, and significant code duplication between ES and OS lineage builders despite shared abstract logic.

⚠️ Bug: Typo 'Store Procedure' should be 'Stored Procedure'

📄 openmetadata-ui/src/main/resources/ui/playwright/utils/common.ts:156

The search index mapping key ['Store Procedure'] is missing the 'd'. The rest of the codebase (e.g., importUtils.ts, class names) consistently uses 'Stored Procedure'. This mismatch will cause entity type lookups to fail for stored procedures, as getEntityTypeSearchIndexMapping('Stored Procedure') will return undefined.

Suggested fix
Change:
    ['Store Procedure']: 'storedProcedure',
To:
    ['Stored Procedure']: 'storedProcedure',
⚠️ Bug: requiresPathPreservation logic changed from OR to AND

The condition for enabling path preservation in buildQueryContext was changed from preservePaths || hasQueryFilter to preservePaths && hasNodeLevelFilters. Previously, any non-empty query filter automatically enabled path preservation (ensuring filtered-out intermediate nodes still had their connecting edges preserved). Now, path preservation requires the caller to explicitly set preservePaths=true and have node-level filters present.

If any caller issues a request with a node-level query filter (e.g., owner, tags) but does not set preservePaths=true, intermediate connecting paths will no longer be preserved. This could cause lineage graphs to appear disconnected when node-level filters remove intermediate nodes that are needed to show the path between the root and matching deeper nodes.

Suggested fix
If the intent is to always preserve paths when node-level filters are active (which was the prior behavior), use OR:

boolean requiresPathPreservation =
    (request.getPreservePaths() != null && request.getPreservePaths())
        || hasNodeLevelFilters;
💡 Quality: Missing afterAll cleanup for table created in beforeAll

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/DataAssetLineage.spec.ts:455-458 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/PlatformLineage.spec.ts:35-38

The newly added test.beforeAll in the 'Lineage Settings modal' block (DataAssetLineage.spec.ts:455-458) creates a TableClass entity but there is no corresponding test.afterAll to delete it. The same issue exists in PlatformLineage.spec.ts (line 35-38) where the table creation was moved to the top-level scope without adding cleanup.

While this follows the existing pattern in the file (no other describe block has afterAll either), it leaves orphaned test entities in the system between runs, which can cause flaky tests or data buildup in CI environments.

Suggested fix
// DataAssetLineage.spec.ts — add after the beforeAll block:
test.afterAll(async ({ browser }) => {
  const { apiContext } = await getDefaultAdminAPIContext(browser);
  await table.delete(apiContext);
  await afterAction();
});

// Same pattern for PlatformLineage.spec.ts
💡 Edge Case: Empty currentDirectionDepthCounts at nodeDepth=0 pagination

When request.getNodeDepth() == 0 and request.getIncludePaginationInfo() is true, buildEntityCountPaginationInfo is called with currentDirectionDepthCounts still set to its initial empty LinkedHashMap. For the requested direction, this means the pagination info will only show depth 0 -> 1 (the root), with no depth counts for deeper levels in that direction. The opposite direction counts are fetched correctly.

This is likely correct behavior (at depth 0 you only see the root), but it means the pagination info won't show how many entities exist at deeper depths for the current direction until the user navigates to a deeper depth. If the intent is to always provide full depth counts in pagination info, the current direction's counts need to be computed even at nodeDepth=0.

💡 Quality: ES and OS lineage builders remain near-identical copies

Despite AbstractLineageGraphBuilder extracting common logic, ESLineageGraphBuilder and OSLineageGraphBuilder still have ~1600 lines each of nearly identical code (e.g., searchLineageByEntityCountInternal, getFilteredDepthCounts, getEntitiesAtSpecificDepthWithPagination, buildEntityCountPaginationInfo, getEntityCountDepthCounts). Every change in this PR had to be applied twice. Consider extracting more shared logic into the abstract base class using template methods, with only the client-specific search execution delegated to subclasses.

✅ 9 resolved
Bug: Incorrect owner patch value format — array of name instead of object with type/id

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:495
At line 495, the owner patch value is [EntityDataClass.user1.responseData.name] (an array containing a string). The API expects an object with type and id fields, as used correctly in all other owner patch calls in this file (e.g., lines 621-624, 766-769).

This will cause the PATCH request to fail or set invalid owner data, making the 'clear all filters' test unreliable.

Bug: Missing await on async entity.patch() call

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:489
At line 489, entity.patch({...}) is called without await. The patch method is async (returns a Promise), so without await the test proceeds before the PATCH request completes. This means the subsequent filter test may run against an entity that hasn't been updated yet, leading to flaky test failures.

All other patch calls in this file (lines 264, 289, 314, 344) correctly use await.

Bug: Schema and column filter tests click wrong dropdown

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:565-578 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:615-628
The 'Verify lineage schema filter selection' test (line ~564) and 'Verify lineage column filter selection' test (line ~614) both click search-dropdown-Database instead of their respective filter dropdowns (e.g., search-dropdown-Schema and search-dropdown-Column). They also read entityResponseData.database.name instead of the appropriate schema/column property. These appear to be copy-pasted from the database filter test without updating the selectors and data paths.

While these tests are marked test.fixme, they'll remain broken even when re-enabled unless the selectors and property paths are corrected.

Quality: LineageRightPanel.spec.ts missing afterAll cleanup for entities

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/LineageRightPanel.spec.ts:44-50 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/LineageRightPanel.spec.ts:101-107
The supportedEntities suite (line 30) creates 10 entities in beforeAll (line 44-49) but has no afterAll hook to delete them. The unsupportedServices suite (line 90) similarly creates 7 services (line 101-107) without cleanup. This leaves test data behind, which can accumulate over repeated test runs and potentially cause flaky behavior in other tests.

Bug: entitiesToHide filter index doesn't match entityToTest index

📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:473 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:476 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:519 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:522 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:569 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:572 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:619 📄 openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineagev1.spec.ts:622
In the new service type, database, schema, and column filter tests, entityToTest is selected at index 5 or 7 in restEntity, but entitiesToHide always filters out index 3 (index !== 3). This means entityToTest will be included in entitiesToHide, and the test will assert it's both visible AND hidden — which will fail (or coincidentally pass for wrong reasons).

For example in the service type filter test: entityToTest = restEntity[5] but entitiesToHide = restEntity.filter((_, index) => index !== 3). The filter should exclude index 5 to match entityToTest.

This is a copy-paste error from the service filter test where the indices were correct (both used 3).

...and 4 more resolved from earlier reviews

🤖 Prompt for agents
Code Review: Test enhancements for lineage specs with comprehensive entity creation and visibility checks. However, four important issues remain: typo 'Store Procedure' should be 'Stored Procedure', critical logic change from OR to AND in path preservation that alters filtering behavior, empty pagination state at nodeDepth=0, and significant code duplication between ES and OS lineage builders despite shared abstract logic.

1. ⚠️ Bug: Typo 'Store Procedure' should be 'Stored Procedure'
   Files: openmetadata-ui/src/main/resources/ui/playwright/utils/common.ts:156

   The search index mapping key `['Store Procedure']` is missing the 'd'. The rest of the codebase (e.g., `importUtils.ts`, class names) consistently uses 'Stored Procedure'. This mismatch will cause entity type lookups to fail for stored procedures, as `getEntityTypeSearchIndexMapping('Stored Procedure')` will return `undefined`.

   Suggested fix:
   Change:
       ['Store Procedure']: 'storedProcedure',
   To:
       ['Stored Procedure']: 'storedProcedure',

2. 💡 Quality: Missing afterAll cleanup for table created in beforeAll
   Files: openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/DataAssetLineage.spec.ts:455-458, openmetadata-ui/src/main/resources/ui/playwright/e2e/Pages/Lineage/PlatformLineage.spec.ts:35-38

   The newly added `test.beforeAll` in the 'Lineage Settings modal' block (DataAssetLineage.spec.ts:455-458) creates a `TableClass` entity but there is no corresponding `test.afterAll` to delete it. The same issue exists in PlatformLineage.spec.ts (line 35-38) where the table creation was moved to the top-level scope without adding cleanup.
   
   While this follows the existing pattern in the file (no other describe block has afterAll either), it leaves orphaned test entities in the system between runs, which can cause flaky tests or data buildup in CI environments.

   Suggested fix:
   // DataAssetLineage.spec.ts — add after the beforeAll block:
   test.afterAll(async ({ browser }) => {
     const { apiContext } = await getDefaultAdminAPIContext(browser);
     await table.delete(apiContext);
     await afterAction();
   });
   
   // Same pattern for PlatformLineage.spec.ts

3. ⚠️ Bug: requiresPathPreservation logic changed from OR to AND

   The condition for enabling path preservation in `buildQueryContext` was changed from `preservePaths || hasQueryFilter` to `preservePaths && hasNodeLevelFilters`. Previously, any non-empty query filter automatically enabled path preservation (ensuring filtered-out intermediate nodes still had their connecting edges preserved). Now, path preservation requires the caller to **explicitly** set `preservePaths=true` **and** have node-level filters present.
   
   If any caller issues a request with a node-level query filter (e.g., owner, tags) but does not set `preservePaths=true`, intermediate connecting paths will no longer be preserved. This could cause lineage graphs to appear disconnected when node-level filters remove intermediate nodes that are needed to show the path between the root and matching deeper nodes.

   Suggested fix:
   If the intent is to always preserve paths when node-level filters are active (which was the prior behavior), use OR:
   
   boolean requiresPathPreservation =
       (request.getPreservePaths() != null && request.getPreservePaths())
           || hasNodeLevelFilters;

4. 💡 Edge Case: Empty currentDirectionDepthCounts at nodeDepth=0 pagination

   When `request.getNodeDepth() == 0` and `request.getIncludePaginationInfo()` is true, `buildEntityCountPaginationInfo` is called with `currentDirectionDepthCounts` still set to its initial empty `LinkedHashMap`. For the requested direction, this means the pagination info will only show `depth 0 -> 1` (the root), with no depth counts for deeper levels in that direction. The opposite direction counts are fetched correctly.
   
   This is likely correct behavior (at depth 0 you only see the root), but it means the pagination info won't show how many entities exist at deeper depths for the current direction until the user navigates to a deeper depth. If the intent is to always provide full depth counts in pagination info, the current direction's counts need to be computed even at nodeDepth=0.

5. 💡 Quality: ES and OS lineage builders remain near-identical copies

   Despite `AbstractLineageGraphBuilder` extracting common logic, `ESLineageGraphBuilder` and `OSLineageGraphBuilder` still have ~1600 lines each of nearly identical code (e.g., `searchLineageByEntityCountInternal`, `getFilteredDepthCounts`, `getEntitiesAtSpecificDepthWithPagination`, `buildEntityCountPaginationInfo`, `getEntityCountDepthCounts`). Every change in this PR had to be applied twice. Consider extracting more shared logic into the abstract base class using template methods, with only the client-specific search execution delegated to subclasses.

Options

Display: compact → Showing less information.

Comment with these commands to change:

Compact
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test Add this label to run secure Github workflows on PRs UI UI specific issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant